Skip to content

test(e2e): run playwright via container#37300

Merged
silverwind merged 15 commits into
go-gitea:mainfrom
TheFox0x7:playwright-on-unsupported
May 10, 2026
Merged

test(e2e): run playwright via container#37300
silverwind merged 15 commits into
go-gitea:mainfrom
TheFox0x7:playwright-on-unsupported

Conversation

@TheFox0x7
Copy link
Copy Markdown
Contributor

Enable running playwright tests on unsupported platforms as well

@silverwind
Copy link
Copy Markdown
Member

I can test this on Arch Linux, which so far was unable to run playwright.

@TheFox0x7
Copy link
Copy Markdown
Contributor Author

I am doing exactly that :)
few webkit and one from firefox timed out but it might be a local thing so I wanted to run it against CI.

unfortunately I think the --network=host is a must because otherwise I need to pass host (which works) but then request fixture can't get the address as it runs on host instead of in container.

Comment thread Makefile Outdated
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 19, 2026
@TheFox0x7 TheFox0x7 force-pushed the playwright-on-unsupported branch from 223c5cd to 61dd882 Compare April 19, 2026 20:04
@lunny
Copy link
Copy Markdown
Member

lunny commented Apr 19, 2026

Wouldn’t that require the developer to install Docker?

@TheFox0x7
Copy link
Copy Markdown
Contributor Author

we have docker target for one. I'm working on skipping the deps install on non-debian/ubuntu installs though and toggling based on that.
Regardless, I think installing container runtime is simpler than changing distro to run tests.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 19, 2026

BTW another approach for limited e2e tests on Arch could be to detect chromium, firefox or webkit binaries in PATH and use that. The first two should work pretty well, webkit will be a gamble. Could make it use binaries on a best-effort basis 😆.

But overall I agree that if docker is available, it's the best fallback.

@silverwind
Copy link
Copy Markdown
Member

#37315 will revert the webkit addition, to unstable. With webkit gone, it may be feasible to run chromium and firefox natively.

@TheFox0x7
Copy link
Copy Markdown
Contributor Author

TheFox0x7 commented Apr 20, 2026

The first two should work pretty well.

Chromium works well. Firefox? I can't get it to work at all because some glib stuff or something. container should be more stable overall for systems not blessed by playwright.
Funnily enough I remember firefox working before so it's probably just a coin toss if it'll work or not.

I'll default to local unless OS is linux and it's not ubuntu/debian. I know I can gate it, just need to fight makefile a bit... though maybe instead I just should write a script like a sane person.


Does makefile and e2e even works under windows?

@silverwind
Copy link
Copy Markdown
Member

Does makefile and e2e even works under windows?

In theory if you have go,node,docker,pnpm available, it may work but I think it's very likely you hit some issue. Real confirmation could only come if we could make CI run on windows runners which I think are not configured.

BTW webkit on macos runners should work reliably but again, I don't think our self-hosted runner infrastructure provides such runners.

@TheFox0x7
Copy link
Copy Markdown
Contributor Author

It was more of a "should I be concerned about it" question. I don't think the test script for e2e would work as it needs bash but I don't exactly have a way of trying it. shell it is as conditionals in makefile are a pain

@silverwind
Copy link
Copy Markdown
Member

No, for all we are concerned, only Linux and MacOS have to work.

@a1012112796
Copy link
Copy Markdown
Member

a1012112796 commented Apr 21, 2026

maybe you can run test in docker by act_runner exec (should temporary update actions configuration file )

image
diff --git a/.github/workflows/pull-e2e-tests.yml b/.github/workflows/pull-e2e-tests.yml
index fde36383b9..2812ee7eb8 100644
--- a/.github/workflows/pull-e2e-tests.yml
+++ b/.github/workflows/pull-e2e-tests.yml
@@ -8,14 +8,14 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  files-changed:
-    uses: ./.github/workflows/files-changed.yml
-    permissions:
-      contents: read
+  # files-changed:
+  #   uses: ./.github/workflows/files-changed.yml
+  #   permissions:
+  #     contents: read
 
   test-e2e:
-    if: needs.files-changed.outputs.backend == 'true' || needs.files-changed.outputs.frontend == 'true' || needs.files-changed.outputs.e2e == 'true'
-    needs: files-changed
+    # if: needs.files-changed.outputs.backend == 'true' || needs.files-changed.outputs.frontend == 'true' || needs.files-changed.outputs.e2e == 'true'
+    # needs: files-changed
     runs-on: ubuntu-latest
     permissions:
       contents: read
@@ -35,7 +35,7 @@ jobs:
       - run: make frontend
       - run: make deps-backend
       - run: make gitea-e2e
-      - run: make playwright
+      - run: GITHUB_ACTIONS="" make playwright
       - run: make test-e2e
         timeout-minutes: 10
         env:

@TheFox0x7
Copy link
Copy Markdown
Contributor Author

But... why? This is just "run in container" but with more steps and requiring act runner, docker daemon and installing browsers which are built-in in the playwright container. I don't follow.

@silverwind
Copy link
Copy Markdown
Member

Goal should be that make test-e2e can run locally on platforms not officially supported by playwright. Either running natively or with a transparent docker wrapper are both acceptable to me, with a preference for native.

Assisted-by: gemini-cli:gemini-3-flash
@TheFox0x7 TheFox0x7 marked this pull request as ready for review April 21, 2026 20:47
@silverwind
Copy link
Copy Markdown
Member

I will test later.

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
@wxiaoguang
Copy link
Copy Markdown
Contributor

Resolved the conflicts caused by my changes.

Just a question: when using container, many test flags like --debug won't work?

@TheFox0x7 TheFox0x7 changed the title run playwright via container test(e2e): run playwright via container May 9, 2026
@TheFox0x7
Copy link
Copy Markdown
Contributor Author

TheFox0x7 commented May 9, 2026

It would seem that they don't unfortunately. I can more or less work around that, just running full suite was basically impossible.

Playwright support on unsupported platforms is pretty bad - who would've guessed :)

@TheFox0x7
Copy link
Copy Markdown
Contributor Author

I meant that I personally can probably work around that issue. It would be just plugging chromium binary to the script and running natively, which is a hack and not worth upstreaming (if it works) as I'd expect majority of people using e2e to run it instead of making new tests, especially on unsupported platforms.

I'll add some comments in a bit.

@silverwind
Copy link
Copy Markdown
Member

Tested on Arch Linux, 54 tests failed with "getaddrinfo ENOTFOUND localhost", so this does seem to depend on "localhost" being dns-resolvable, which may be some debian/ubuntu specific thing. After adding 127.0.0.1 localhost to /etc/hosts, it worked.

@silverwind
Copy link
Copy Markdown
Member

Tested on Arch Linux, 54 tests failed with "getaddrinfo ENOTFOUND localhost", so this does seem to depend on "localhost" being dns-resolvable, which may be some debian/ubuntu specific thing. After adding 127.0.0.1 localhost to /etc/hosts, it worked.

Apparently this was a misconfiguration on my side, https://wiki.archlinux.org/title/Network_configuration#:~:text=local%20hostname

- guard `docker stop` in cleanup with `|| true` so a failed stop doesn't
  skip server kill / workdir removal
- use a unique container name per process (`gitea-e2e-runner-$$`) so
  concurrent runs don't collide
- drop `-it` from `docker run` (meaningless with `-d`)
- strip leading semver range from PLAYWRIGHT_VERSION so a `^`/`~` in
  package.json doesn't break the image tag
- normalize indentation to 2 spaces (matches .editorconfig)
- fail fast with a clear message if container mode is selected but the
  runtime is not installed
- drop unconditional 5s sleep in wait_for_container; poll immediately

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member

Pushed some cleanups in 8134227.

Comment thread tools/test-e2e.sh Outdated
Comment thread Makefile Outdated
- normalise CONTAINER_RUNTIME once at the top of the script and drop the
  `?= docker` default + recipe passthrough from the Makefile (env vars
  inherit; command-line make vars auto-export)
- replace `node` invocation for version extraction with `sed`
- extract a shared `free_port` helper and use it for both the gitea
  server port and a dynamically allocated playwright run-server port
- rewrite `wait_for_container` to probe the playwright TCP port via
  bash's `/dev/tcp` instead of grepping `docker logs`; fixed 1s polling
  with a 30s budget; dump container logs on timeout
- skip `docker pull` when the image is already present locally
- drop narration comments

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Comment thread tools/test-e2e.sh Outdated
Signed-off-by: silverwind <me@silverwind.io>
Comment thread tools/test-e2e.sh Outdated
- guard `shift` so direct invocation of the script with no args doesn't
  exit under `set -e`
- validate the parsed @playwright/test version against a strict
  semver-ish regex; rejects empty / malformed values and prevents shell
  injection into the container's `sh -c` command
- detect early container exit inside the readiness loop and dump logs
  immediately instead of waiting out the 30s budget
- drop the container-mode timeout-factor bump; the existing CI=4 / 1
  buckets plus the manual override env var cover the real cases
- move PLAYWRIGHT_SERVER_PORT allocation out of the install path
- trim narration comments

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 10, 2026
Comment thread tools/test-e2e.sh Outdated
silverwind and others added 3 commits May 10, 2026 10:49
Signed-off-by: silverwind <me@silverwind.io>
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't really tested on linux, overall code looks good.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 10, 2026
@silverwind
Copy link
Copy Markdown
Member

Tested latest commit on Arch, works.

@silverwind silverwind enabled auto-merge (squash) May 10, 2026 08:58
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 10, 2026
@silverwind silverwind merged commit c78c84c into go-gitea:main May 10, 2026
21 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone May 10, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 10, 2026
silverwind added a commit to KalashThakare/gitea that referenced this pull request May 10, 2026
* origin/main:
  feat(api): add last_sync to repository API (go-gitea#37566)
  test(e2e): run playwright via container (go-gitea#37300)

# Conflicts:
#	models/migrations/migrations.go
#	models/migrations/v1_27/v332.go
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 12, 2026
* main:
  fix(deps): update dependency mermaid to v11.15.0 [security], add e2e test (go-gitea#37662)
  ci: Also lint json5 files (go-gitea#37659)
  fix(templates): avoid misleading compare message when branches lack merge base (go-gitea#37651)
  fix(deps): update npm dependencies (go-gitea#37647)
  refactor: routing info middleware (go-gitea#37653)
  chore(deps): update action dependencies (major) (go-gitea#37638)
  fix(deps): update go dependencies (major) (go-gitea#37639)
  ci(renovate): update Go import paths on major bumps (go-gitea#37641)
  fix(packages): Add label for private and internal package and fix composor package source permission check (go-gitea#37610)
  refactor: replace Fomantic search module with first-party code (go-gitea#37443)
  fix(deps): update npm dependencies (go-gitea#37636)
  fix(deps): update module code.gitea.io/sdk/gitea to v0.25.0 (go-gitea#37637)
  feat(api): add last_sync to repository API (go-gitea#37566)
  test(e2e): run playwright via container (go-gitea#37300)
  feat(editor): broaden language detection in web code editor (go-gitea#37619)
  refactor(log): replace log.Critical with log.Error (go-gitea#37624)
  fix: "run as root" check (go-gitea#37622)
  fix: improve actions status icons and texts (go-gitea#37206)

# Conflicts:
#	pnpm-workspace.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants